Skip to content

Handle vendored descriptor.proto gracefully in lowering#710

Merged
emcfarlane merged 9 commits intomainfrom
ed/partialLower
Apr 17, 2026
Merged

Handle vendored descriptor.proto gracefully in lowering#710
emcfarlane merged 9 commits intomainfrom
ed/partialLower

Conversation

@emcfarlane
Copy link
Copy Markdown
Contributor

@emcfarlane emcfarlane commented Apr 16, 2026

Builds on #699, which replaced panics with a single valid flag. That flag was too coarse: any missing symbol blocked lowering for every file in the session, so a proto2/proto3 file against a pre-editions vendored descriptor.proto would fail compilation even though it used nothing editions-era.

Changes:

  • Widen builtin:"optional" to cover everything introduced in editions 2023+: FeatureSet, all Feature* / Features, OptionTargets, UnverifiedLazy, EditionDefaults, EditionSupport*, ExtnDecls*.
  • Replace the valid flag with one error diagnostic per missing required symbol, attached to descriptor.proto. Removes File.Lowered().
  • IsClosedEnum / IsPacked / IsUnicode / populateJSONNames use syntax for proto2/proto3 and only consult FeatureSet for editions.
  • Augment the "cannot find message field" diagnostic when the name matches a tagged-optional builtin, pointing at descriptor.proto and explaining it's editions-era.

Four fixtures in experimental/ir/testdata/builtins/ cover broken, vendored-proto3 (existing), vendored-proto3 with options, and vendored-editions with unavailable feature.

Copy link
Copy Markdown
Member

@doriable doriable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks generally good -- I left a note to include the symbol tables for the test cases, but am fine to approve ahead of those being generated. Thank you!

@emcfarlane emcfarlane marked this pull request as ready for review April 17, 2026 08:45
@emcfarlane emcfarlane merged commit 65c782f into main Apr 17, 2026
10 checks passed
@emcfarlane emcfarlane deleted the ed/partialLower branch April 17, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants